refactor: use `#` for private methods to reduce the minified file size #3596

yusukebe · 2024-10-31T07:40:07Z

This PR reduces the bundle size using # for the private methods instead of the private keyword when the code is minified.

The result for the "Hello World" app with the hono/tiny preset. This is minified with esbuild --minify command:

-rw-r--r--@  1 yusuke  staff  12167 10 31 16:13 current.js
-rw-r--r--@  1 yusuke  staff  11688 10 31 16:18 next.js

479 bytes will be reduced without changing any logic and without performance degradation!

`#` vs `private`

For example, you have the code:

class A {
  private myPrivateMethod() {}
  myPublicMethod() {
    this.myPrivateMethod()
  }
}

If you use the private keyword for the private method, the function name is not minifed with esbuild

"use strict";class A{myPrivateMethod(){}myPublicMethod(){this.myPrivateMethod()}}

Instead of private, you can use # for the private method.

class A {
  #myPrivateMethod() {}
  myPublicMethod() {
    this.#myPrivateMethod()
  }
}

Then, the function name will be minified!

"use strict";class A{#t(){}myPublicMethod(){this.#t()}}

codecov · 2024-10-31T07:42:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.74%. Comparing base (01277aa) to head (96006d7).
Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3596      +/-   ##
==========================================
- Coverage   94.71%   89.74%   -4.97%     
==========================================
  Files         158      158              
  Lines        9552    10086     +534     
  Branches     2825     2809      -16     
==========================================
+ Hits         9047     9052       +5     
- Misses        505     1034     +529

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yusukebe · 2024-10-31T07:45:01Z

Hi @usualoma @EdamAme-x

What do you think of this?

EdamAme-x · 2024-10-31T07:58:44Z

I think it's a nice idea.
If we focus only on the core package, the other thing that could be changed is the private of routers.

https://github.com/search?q=repo%3Ahonojs%2Fhono+lang%3ATypeScript++%22private+%22+path%3A%2F%5Esrc%5C%2Frouter%5C%2F%2F&type=code

yusukebe · 2024-10-31T08:01:09Z

I'm not sure how much this change will affect real world examples, but parsing time may be reduced, and the file upload time to the production environment will be faster, although super slightly.

yusukebe · 2024-10-31T08:09:28Z

@EdamAme-x

Good idea! Updated 24d4fb1

usualoma · 2024-10-31T11:54:53Z

Hi @yusukebe, I think it's a really good refactoring 👍

How to define instance methods

This is another refactoring story, but instance methods are faster to create instances if they are written directly than if they are defined by assignment.
In the very simple case, there is almost no overhead in deno, so the latest v8 may be optimized to make no difference. However, the #method = this.otherMethod pattern has a non-trivial overhead even in deno.

Given this background, we would like to carefully consider the following line of approach.
https://github.com/honojs/hono/pull/3596/files#diff-0cd594dddd6a9f2e3e26afebbb92716434437d7001c2416252aa7531e7f2b8d4R693

import { run, bench, group } from 'mitata'

bench('noop', () => {})

class A {
  method = () => {}
  #method2 = this.method
}

class B {
  method() {
    // noop
  }
  #method2() {
    this.method()
  }
}

group('create instance', () => {
  bench('class A', () => new A())
  bench('class B', () => new B())
})

await run()

cpu: Apple M2 Pro
runtime: node v22.2.0 (arm64-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop           69 ps/iter        (20 ps … 379 ns)     81 ps    102 ps    204 ps !

• create instance
------------------------------------------------- -----------------------------
class A       168 ns/iter       (158 ns … 269 ns)    166 ns    223 ns    260 ns
class B       112 ps/iter        (61 ps … 117 ns)    122 ps    163 ps    204 ps !

summary for create instance
  class B
   1498.52x faster than class A

cpu: Apple M2 Pro
runtime: deno 2.0.0 (aarch64-apple-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop          498 ps/iter     (427 ps … 34.95 ns)    509 ps    610 ps    753 ps

• create instance
------------------------------------------------- -----------------------------
class A      1.04 ns/iter       (752 ps … 161 ns)   1.04 ns   1.42 ns  31.86 ns
class B       549 ps/iter     (488 ps … 44.05 ns)    549 ps    610 ps    773 ps

summary for create instance
  class B
   1.89x faster than class A

cpu: Apple M2 Pro
runtime: bun 1.1.30 (arm64-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop           44 ps/iter       (0 ps … 19.84 ns)     41 ps     82 ps    102 ps !

• create instance
------------------------------------------------- -----------------------------
class A     33.01 ns/iter     (28.83 ns … 490 ns)  31.07 ns    163 ns    229 ns
class B        46 ps/iter         (0 ps … 279 ns)     41 ps     61 ps    122 ps !

summary for create instance
  class B
   711.59x faster than class A

yusukebe · 2024-11-01T08:43:33Z

@usualoma

Thanks! I was wondering how to define the #newResponse. How about this "class B" style?

7f05f7d

usualoma · 2024-11-01T13:19:46Z

@yusukebe Thank you!

benchmark - 2

Here is a version of the benchmark with a few additions.

Add class C to define and assign functions outside
Not only new, but also method() calls

The results are almost the same for class B and class C in any runtime, with class A performing worse, maybe JIT is less effective or something.

import { run, bench, group } from 'mitata'

bench('noop', () => {})

class A {
  method = () => {}
}

class B {
  method() {
    // noop
  }
}

const method = () => {}
class C {
  method = method
}

group('create instance', () => {
  bench('class A', () => new A().method())
  bench('class B', () => new B().method())
  bench('class C', () => new C().method())
})

run()

cpu: Apple M2 Pro
runtime: node v22.2.0 (arm64-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop           70 ps/iter        (20 ps … 339 ns)     82 ps     82 ps    163 ps !

• create instance
------------------------------------------------- -----------------------------
class A       168 ns/iter       (162 ns … 268 ns)    167 ns    212 ns    263 ns
class B       110 ps/iter      (61 ps … 67.81 ns)    122 ps    122 ps    183 ps !
class C       112 ps/iter        (61 ps … 146 ns)    122 ps    143 ps    203 ps !

summary for create instance
  class B
   1.02x faster than class C
   1525.21x faster than class A

cpu: Apple M2 Pro
runtime: deno 2.0.0 (aarch64-apple-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop          499 ps/iter     (427 ps … 22.91 ns)    509 ps    590 ps    732 ps

• create instance
------------------------------------------------- -----------------------------
class A      2.48 ns/iter      (1.85 ns … 188 ns)   2.36 ns   4.35 ns  36.01 ns
class B       549 ps/iter       (488 ps … 119 ns)    549 ps    611 ps   1.65 ns
class C       556 ps/iter     (488 ps … 72.79 ns)    550 ps    692 ps   1.44 ns

summary for create instance
  class B
   1.01x faster than class C
   4.52x faster than class A

cpu: Apple M2 Pro
runtime: bun 1.1.30 (arm64-darwin)

benchmark      time (avg)             (min … max)       p75       p99      p999
------------------------------------------------- -----------------------------
noop           49 ps/iter       (0 ps … 19.88 ns)     61 ps     82 ps    122 ps !

• create instance
------------------------------------------------- -----------------------------
class A     33.61 ns/iter     (29.07 ns … 302 ns)   31.7 ns    169 ns    234 ns
class B        48 ps/iter         (0 ps … 205 ns)     41 ps     61 ps    122 ps !
class C        45 ps/iter         (0 ps … 178 ns)     41 ps     61 ps    122 ps !

summary for create instance
  class C
   1.06x faster than class B
   739.63x faster than class A

c.newRespons vs. c.#newRespons

c.newResponse is not called in many apps. On the other hand, c.#newResponse is often called multiple times internally. Therefore, I think it would be better to prioritize improving the performance of c.#newResponse.
Also, even though it is for internal use, I think it would be better to add some type to c.#newResponse.

For the reasons given above, why not give c.#newResponse priority with the following changes?

diff --git a/src/context.ts b/src/context.ts
index a591db72..363b6556 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -624,11 +624,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }
 
-  newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {
@@ -690,10 +690,7 @@ export class Context<
     })
   }
 
-  #newResponse(...args: unknown[]) {
-    // @ts-expect-error Type mismatch for args in newResponse call
-    return this.newResponse(...args)
-  }
+  newResponse: NewResponse = (...args) => this.#newResponse(...(args as Parameters<NewResponse>))
 
   /**
    * `.body()` can return the HTTP response.

yusukebe · 2024-11-02T02:09:13Z

@usualoma

For the reasons given above, why not give c.#newResponse priority with the following changes?

That's a very interesting idea!! I've embraced it: 3ac8d62

I think we can go now. Can you review this again?

usualoma · 2024-11-02T02:33:38Z

@yusukebe Thank you!

Do you think this change is something you don't want to add? (Because it's a change that's in my comments but not in 3ac8d62)

Well, it may not be necessary, but from the benchmark, performance decreases with each additional #method = () => {}, so I think it's better to use #method() {} for hard private methods, even if it means compromising on some typing.

diff --git a/src/context.ts b/src/context.ts
index 3bb9ceda..86124e00 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -623,11 +623,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }
 
-  #newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {

yusukebe · 2024-11-02T02:38:35Z

@usualoma Thanks!

Or like this?

diff --git a/src/context.ts b/src/context.ts
index 3bb9ceda..6a2f8f47 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -623,11 +623,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }

-  #newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {
@@ -689,7 +689,9 @@ export class Context<
     })
   }

-  newResponse: NewResponse = (...args) => this.#newResponse(...(args as Parameters<NewResponse>))
+  newResponse(...args: Parameters<NewResponse>) {
+    return this.#newResponse(...args)
+  }

   /**
    * `.body()` can return the HTTP response.

usualoma · 2024-11-02T10:27:50Z

@yusukebe Thank you for your consideration.

I think that would mean that the intended type would no longer be available for the public API newResponse().

yusukebe · 2024-11-02T10:42:00Z

@usualoma

Thanks! Probably, the type definition is not good. How about changing interface NewResponse?

diff --git a/src/context.ts b/src/context.ts
index 3bb9ceda..3bf30cef 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -107,8 +107,7 @@ interface Set<E extends Env> {
  * Interface for creating a new response.
  */
 interface NewResponse {
-  (data: Data | null, status?: StatusCode, headers?: HeaderRecord): Response
-  (data: Data | null, init?: ResponseInit): Response
+  (data: Data | null, arg?: StatusCode | ResponseInit, headers?: HeaderRecord): Response
 }

 /**
@@ -623,11 +622,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }

-  #newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {
@@ -689,7 +688,9 @@ export class Context<
     })
   }

-  newResponse: NewResponse = (...args) => this.#newResponse(...(args as Parameters<NewResponse>))
+  newResponse(...args: Parameters<NewResponse>) {
+    return this.#newResponse(...args)
+  }

   /**
    * `.body()` can return the HTTP response.

usualoma · 2024-11-02T11:59:13Z

@yusukebe Thank you!

I don't think that's what we wanted to do with the original definition, because it would accept the following.

If we prioritize performance over being DRY, I think the only way to write it is as follows

diff --git a/src/context.ts b/src/context.ts
index 3bb9ceda..cd60ef1f 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -623,11 +623,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }
 
-  #newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {
@@ -689,7 +689,13 @@ export class Context<
     })
   }
 
-  newResponse: NewResponse = (...args) => this.#newResponse(...(args as Parameters<NewResponse>))
+  newResponse(data: Data | null, status?: StatusCode, headers?: HeaderRecord): Response
+  newResponse(data: Data | null, init?: ResponseInit): Response
+  newResponse(
+    ...args: [data: Data | null, arg?: StatusCode | ResponseInit, headers?: HeaderRecord]
+  ): Response {
+    return this.#newResponse(...args)
+  }
 
   /**
    * `.body()` can return the HTTP response.

Or, we could go one step further and do this.

diff --git a/src/context.ts b/src/context.ts
index 3bb9ceda..72069c1e 100644
--- a/src/context.ts
+++ b/src/context.ts
@@ -103,19 +103,6 @@ interface Set<E extends Env> {
   <Key extends keyof ContextVariableMap>(key: Key, value: ContextVariableMap[Key]): void
 }
 
-/**
- * Interface for creating a new response.
- */
-interface NewResponse {
-  (data: Data | null, status?: StatusCode, headers?: HeaderRecord): Response
-  (data: Data | null, init?: ResponseInit): Response
-}
-
-/**
- * Interface for responding with a body.
- */
-interface BodyRespond extends NewResponse {}
-
 /**
  * Interface for responding with text.
  *
@@ -623,11 +610,11 @@ export class Context<
     return Object.fromEntries(this.#var)
   }
 
-  #newResponse: NewResponse = (
+  #newResponse(
     data: Data | null,
     arg?: StatusCode | ResponseInit,
     headers?: HeaderRecord
-  ): Response => {
+  ): Response {
     // Optimized
     if (this.#isFresh && !headers && !arg && this.#status === 200) {
       return new Response(data, {
@@ -689,7 +676,13 @@ export class Context<
     })
   }
 
-  newResponse: NewResponse = (...args) => this.#newResponse(...(args as Parameters<NewResponse>))
+  newResponse(data: Data | null, status?: StatusCode, headers?: HeaderRecord): Response
+  newResponse(data: Data | null, init?: ResponseInit): Response
+  newResponse(
+    ...args: [data: Data | null, arg?: StatusCode | ResponseInit, headers?: HeaderRecord]
+  ): Response {
+    return this.#newResponse(...args)
+  }
 
   /**
    * `.body()` can return the HTTP response.
@@ -712,11 +705,9 @@ export class Context<
    * })
    * ```
    */
-  body: BodyRespond = (
-    data: Data | null,
-    arg?: StatusCode | ResponseInit,
-    headers?: HeaderRecord
-  ): Response => {
+  body(data: Data | null, init?: ResponseInit): Response
+  body(data: Data | null, status?: StatusCode, headers?: HeaderRecord): Response
+  body(data: Data | null, arg?: StatusCode | ResponseInit, headers?: HeaderRecord): Response {
     return typeof arg === 'number'
       ? this.#newResponse(data, arg, headers)
       : this.#newResponse(data, arg)

usualoma · 2024-11-02T12:10:54Z

I think the proposal described in the following comments is a good compromise that does not involve major changes to the current code and is unlikely to cause a drop in performance.

#3596 (comment)

yusukebe · 2024-11-02T22:46:10Z

@usualoma

Sorry to bother you. Let's go as you suggested. Can you review it?

usualoma

Very refactoring 👍

yusukebe · 2024-11-02T23:31:52Z

Okay! Merging now.

refactor: use # for private methods to reduce the bundle size

311b078

yusukebe changed the title ~~refactor: use # for private methods to reduce the bundle size~~ refactor: use # for private methods to reduce the minified bundle size Oct 31, 2024

yusukebe changed the title ~~refactor: use # for private methods to reduce the minified bundle size~~ refactor: use # for private methods to reduce the minified file size Oct 31, 2024

use # for routers

24d4fb1

defined #newResponse with a method style

7f05f7d

yusukebe added 2 commits November 2, 2024 10:52

Merge branch 'main' into refactor/use-sharp

890f16b

Swap c.#newResponse and c.newResponse

3ac8d62

make #newResponse as #method() {} style

96006d7

yusukebe requested a review from usualoma November 2, 2024 22:45

usualoma approved these changes Nov 2, 2024

View reviewed changes

yusukebe merged commit ae6165b into main Nov 2, 2024
16 checks passed

yusukebe deleted the refactor/use-sharp branch November 2, 2024 23:32

smoothdvd mentioned this pull request Nov 5, 2024

Hono 4.6.9: Cannot read private member #errorHandler felixmosh/bull-board#843

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use `#` for private methods to reduce the minified file size #3596

refactor: use `#` for private methods to reduce the minified file size #3596

yusukebe commented Oct 31, 2024 •

edited

Loading

codecov bot commented Oct 31, 2024 •

edited

Loading

yusukebe commented Oct 31, 2024

EdamAme-x commented Oct 31, 2024 •

edited

Loading

yusukebe commented Oct 31, 2024

yusukebe commented Oct 31, 2024

usualoma commented Oct 31, 2024

yusukebe commented Nov 1, 2024

usualoma commented Nov 1, 2024

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma left a comment

yusukebe commented Nov 2, 2024

refactor: use # for private methods to reduce the minified file size #3596

refactor: use # for private methods to reduce the minified file size #3596

Conversation

yusukebe commented Oct 31, 2024 • edited Loading

# vs private

codecov bot commented Oct 31, 2024 • edited Loading

Codecov Report

yusukebe commented Oct 31, 2024

EdamAme-x commented Oct 31, 2024 • edited Loading

yusukebe commented Oct 31, 2024

yusukebe commented Oct 31, 2024

usualoma commented Oct 31, 2024

How to define instance methods

yusukebe commented Nov 1, 2024

usualoma commented Nov 1, 2024

benchmark - 2

c.newRespons vs. c.#newRespons

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma commented Nov 2, 2024

usualoma commented Nov 2, 2024

yusukebe commented Nov 2, 2024

usualoma left a comment

Choose a reason for hiding this comment

yusukebe commented Nov 2, 2024

refactor: use `#` for private methods to reduce the minified file size #3596

refactor: use `#` for private methods to reduce the minified file size #3596

yusukebe commented Oct 31, 2024 •

edited

Loading

`#` vs `private`

codecov bot commented Oct 31, 2024 •

edited

Loading

EdamAme-x commented Oct 31, 2024 •

edited

Loading